Skip to content
This repository was archived by the owner on Nov 14, 2018. It is now read-only.

Removed generation of double Optionals for properties of Input type#9

Open
MarioBajr wants to merge 1 commit intoamazon-archives:masterfrom
EFEducationFirstMobile:fixes/removedDoubleOptionals
Open

Removed generation of double Optionals for properties of Input type#9
MarioBajr wants to merge 1 commit intoamazon-archives:masterfrom
EFEducationFirstMobile:fixes/removedDoubleOptionals

Conversation

@MarioBajr
Copy link

@MarioBajr MarioBajr commented Jul 4, 2018 •
edited
Loading

For objects of type Input, the Swift code generation is creating unnecessary layers of Optionality for all Optional properties.

Given this GraphQL input type

input ReviewInput {
    stars: Int!
    commentary: String
    favoriteColor: ColorInput
}

The code generated has two layers of Optionality. eg: Optional<String?>

public struct ReviewInput: GraphQLMapConvertible {
    public init(stars: Int, 
                commentary: Optional<String?> = nil, 
                favoriteColor: Optional<ColorInput?> = nil) {
        ...
    }
}

This PR replaces to a single Optionality level, aligning with all other generated types.

public struct ReviewInput: GraphQLMapConvertible {
    public init(stars: Int, 
                commentary: String? = nil, 
                favoriteColor: ColorInput? = nil) {
        ...
    }
}

@minbi
Copy link
Contributor

minbi commented Jul 5, 2018

Thanks @MarioBajr ,

I will discuss with the team about the change and update you with the decision.

@frankmuellr
Copy link

frankmuellr commented Oct 3, 2018

Hello @MarioBajr

We will be retiring this repo soon as the new [AWS Amplify CLI](url
https://github.com/aws-amplify/amplify-cli) has the codegen features, improvements, and more, including automatic graphql generation as well as a GraphQL transformer:
https://github.com/aws-amplify/amplify-cli/blob/master/native_guide.md

Could you look to see if the new AWS Amplify CLI flow resolves your issue and if not open something in that repo?

@MarioBajr
Copy link
Author

MarioBajr commented Oct 4, 2018 •
edited
Loading

Hi @muellerfr,
thanks for the updates, while evaluating amplify-cli, I found that this project (aws-appsync-codegen) is imported as a dependency, being the responsible for the Swift code generation.
https://github.com/aws-amplify/amplify-cli/blob/master/packages/amplify-codegen/package.json#L17
https://github.com/aws-amplify/amplify-cli/blob/master/packages/amplify-codegen/src/commands/types.js#L39

Is the new codegen feature that you guys are planning to use available anywhere else?
Cheers

@rohandubal rohandubal self-requested a review November 14, 2018 01:08
@rohandubal rohandubal self-assigned this Nov 14, 2018
@frankmuellr
Copy link

frankmuellr commented Nov 14, 2018

Hi @MarioBajr, we will merge this change into the https://github.com/aws-amplify/amplify-cli repo, where this code now lives.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@rohandubal rohandubal rohandubal approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@MarioBajr @minbi @frankmuellr @rohandubal

Comments